-
Notifications
You must be signed in to change notification settings - Fork 2
Analytics test #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Analytics test #20
Conversation
core/caddy/Caddyfile
Outdated
| replace "</body>" "<script src='https://performance.zoo/shared.js' async defer></script></body>" | ||
| replace "</BODY>" "<script src='https://performance.zoo/shared.js' async defer></script></BODY>" | ||
| replace "</body>" "<script src='http://performance.zoo/shared.js' async defer></script></body>" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could just use //performance.zoo/shared.js here so it inherits from the page it's being embedded in. Would this solve the snappymail issue you mention @marimeireles or is that separate?
Also, are the cert errors you're seeing due to the browser not having the cert imported (so no https works at all, even page loads), or is it specific to this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https seem to work, pages are loading, as long as I manually "accept the risk". Am I missing something in the config?
Changing the URL didn't work for the certificates problem.
My understanding is that the problem I was having with snappymail was CSP one.
I was able to remove it from the page and now I'm getting cache issues! :/ I can't seem to make the page to load the inject shared.js. Not sure what's the problem, I've erased the snappy cache manually and still get this cached page?
I'll take a look at it later. I guess I'll focus on adding the other pages next.
|
Thanks! I'm inclined to not worry about persistence (and instead create the analytics db just like any other in https://github.com/bgrins/the_zoo/blob/ae844f927bfb529e067674fa808ac66ba3a5fe94/core/mysql/init-databases.sh). In the case where persistence is important, and external harness could manage dumping and aggregating results across restarts. That would simplify the patch and marginally help with resource usage at runtime without an extra db container. |
|
Makes sense! I'm working on this rn, will probably have an up to date PR by the end of my CET day! |
d50b10a to
2f49c82
Compare
| "**/tests/*.skip.js", | ||
| "**/tests/tools/**", | ||
| "**/tests/fresh/**", | ||
| "**/tests/playwright/**", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the other playwright tests might be broken. We can work on that separately, but in the meantime suggest you keep the exclusion here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to keep the new playwright test in the patch, we can figure out getting it running later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright sounds good.
| - MATOMO_DATABASE_DBNAME=analytics_db | ||
| - MATOMO_DATABASE_TABLES_PREFIX=matomo_ | ||
| - PHP_MEMORY_LIMIT=256M | ||
| volumes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've typically not mounted volumes like these since we don't want data persisted across restarts. Have you found this to be necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
I want users to have access to matomo's database of websites across restarts.
Otherwise every time you spin an instance of it one has to use the MCP (that hasn't worked to do that before, I had to do it manually) in order to add the websites instances into the matomo GUI.
| echo " Size: $(du -h core/mysql/sql/analytics_seed.sql | cut -f1)" | ||
|
|
||
| echo "" | ||
| echo "2️⃣ Capturing Matomo application data..." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this config folder contain seed-specific state? We have to do that on gitea bc it stores the actual repos on disc, but for many sites it's not necessary (if the web server is more or less stateless)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see some specific stuff in config.ini.php. If that doesn't get auto generated at startup maybe that's all we need to include in the golden dir? I'm fine to land and optimize later if we know at least config is necessary to capture
|
Looks great @marimeireles, and I can even see the Behavior reporting working. Just a couple things before it lands:
After that it looks like tests should be passing and we can get it landed. We can figure out miniflux and any additional testing as followups. |
bf2aef5 to
821d8ad
Compare
Introduces analytics.
This is just an MVP in which I've added analytics for the search and tried to add it for email. I'm posting it early in case this is not going in the direction you imagined, Brian.
Known issues:
Let me know what you think, I hope to have more time to work on this this week!
Best,